Show Claude icon in terminal header while Claude Code is active#3046
Show Claude icon in terminal header while Claude Code is active#3046
Conversation
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Deploying waveterm with
|
| Latest commit: |
0a88565
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4f90dfe0.waveterm.pages.dev |
| Branch Preview URL: | https://copilot-add-header-icon-for.waveterm.pages.dev |
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
The incremental changes fix both previous issues:
Test updates confirm the fixes work correctly. Reviewed by minimax-m2.5-20260211 · 186,201 tokens |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Status: No Issues Found | Recommendation: Merge
Files Reviewed (10 files)
emain/emain-ipc.ts- Null safety fix for webContents IDemain/emain-tabview.ts- Null checks and caching improvementsemain/emain-util.ts- PDF viewer frame navigation fixemain/emain-window.ts- Window positioning refactorfrontend/app/view/term/fitaddon.ts- WebGL support detection fix (webgl2)frontend/app/view/term/term-model.ts- WebGL toggle UI and IME handling removalfrontend/app/view/term/term.scss- Scrollbar styling removalfrontend/app/view/term/term.tsx- Overview ruler configfrontend/app/view/term/termwrap.ts- WebGL dynamic toggle, IME composition removalfrontend/app/view/term/xterm.css- CSS formatting and scrollbar stylingfrontend/app/view/webview/webview.tsx- WaveEnv integration refactorfrontend/app/view/webview/webview.test.tsx- New tests
The changes implement:
- Dynamic WebGL rendering toggle with UI indicator
- WebView WaveEnv-based refactoring for preview support
- Null safety improvements in IPC handlers
- PDF viewer frame navigation improvements
- Tab initialization refactoring
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes add Claude Code command detection functionality across the terminal view system. A new regex-based detection function Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app/view/term/osc-handlers.ts (1)
111-123:⚠️ Potential issue | 🟠 MajorClear Claude-active state when
cmd64is rejected for size.In the oversized command branch,
claudeCodeActiveAtomis not reset. If the previous command was Claude, the header can stay stale until a later lifecycle event.🔧 Suggested fix
if (decodedLen > 8192) { rtInfo["shell:lastcmd"] = `# command too large (${decodedLen} bytes)`; globalStore.set(termWrap.lastCommandAtom, rtInfo["shell:lastcmd"]); + globalStore.set(termWrap.claudeCodeActiveAtom, false); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/view/term/osc-handlers.ts` around lines 111 - 123, When handling the oversized cmd64 branch in the if (cmd.data.cmd64) block, also clear the Claude-active state and related atoms so a previous Claude command doesn't remain stale: in the decodedLen > 8192 branch set rtInfo["shell:lastcmd"] as already done and call globalStore.set(termWrap.claudeCodeActiveAtom, false) (and any other related atom/state if present) to explicitly reset the Claude-active flag; ensure this mirrors the non-oversized path where isClaudeCodeCommand(decodedCmd) updates claudeCodeActiveAtom and checkCommandForTelemetry is called.
🧹 Nitpick comments (1)
frontend/app/view/term/osc-handlers.test.ts (1)
12-20: Add regression cases forenv KEY=... claudeand path-argument false positives.Current coverage misses two important edge cases that affect detection correctness:
env ANTHROPIC_API_KEY=test claude(should betrue) andecho /usr/local/bin/claude(should befalse).✅ Suggested test additions
describe("isClaudeCodeCommand", () => { @@ it("matches Claude Code invocations wrapped with env assignments", () => { expect(isClaudeCodeCommand('ANTHROPIC_API_KEY="test" claude')).toBe(true); expect(isClaudeCodeCommand("FOO=bar env claude --print")).toBe(true); + expect(isClaudeCodeCommand("env ANTHROPIC_API_KEY=test claude")).toBe(true); }); @@ it("ignores other commands", () => { expect(isClaudeCodeCommand("claudes")).toBe(false); expect(isClaudeCodeCommand("echo claude")).toBe(false); + expect(isClaudeCodeCommand("echo /usr/local/bin/claude")).toBe(false); expect(isClaudeCodeCommand("")).toBe(false); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/view/term/osc-handlers.test.ts` around lines 12 - 20, Tests currently miss two edge cases: commands prefixed with "env KEY=... claude" and path-argument false positives like "echo /usr/local/bin/claude". Update the test file (osc-handlers.test.ts) to add assertions calling isClaudeCodeCommand("env ANTHROPIC_API_KEY=test claude") expecting true and isClaudeCodeCommand("echo /usr/local/bin/claude") expecting false; if these new tests fail, adjust the detection logic in isClaudeCodeCommand to treat "env" style env assignments as valid invocations and to ignore occurrences where "claude" appears as part of a path or as an argument to echo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/view/term/osc-handlers.ts`:
- Around line 28-29: The current Claude detection uses ClaudeCodeRegex
(/?:^|\/)claude\b/ ) against the entire command string which produces false
positives; update the code so detection runs only against the command token
(argv[0]) instead of the full command text — i.e., extract the command token
(e.g., const cmd = argv[0] or the variable already holding the command name) and
apply ClaudeCodeRegex to cmd, and make the same change wherever ClaudeCodeRegex
is used (including the other occurrence around lines 91–96) so only the
executable name/path is checked.
- Around line 48-52: normalizeCmd fails to strip forms like "env KEY=val claude"
because it only removes a leading "env " token; update normalizeCmd to remove an
"env" wrapper plus any following KEY=... assignments before the command.
Specifically, modify the second replacement (the one handling /^env\s+/ in
normalizeCmd) to match /^env(?:\s+\w+=(?:"[^"]*"|'[^']*'|\S+))*\s+/ so it strips
"env" and zero-or-more assignment tokens that follow, then returns the remaining
command (so Claude detection will see "claude" at the start).
In `@frontend/app/view/term/termwrap.ts`:
- Around line 176-199: The OSC handler catch blocks currently return false which
relinquishes ownership on errors; update the three handlers registered with
registerOscHandler(7), registerOscHandler(52), and registerOscHandler(16162) so
their catch blocks return true instead of false, keeping ownership when
handleOsc7Command, handleOsc52Command, or handleOsc16162Command throw; keep the
existing logging (console.error) and otherwise leave the try/catch behavior
unchanged.
---
Outside diff comments:
In `@frontend/app/view/term/osc-handlers.ts`:
- Around line 111-123: When handling the oversized cmd64 branch in the if
(cmd.data.cmd64) block, also clear the Claude-active state and related atoms so
a previous Claude command doesn't remain stale: in the decodedLen > 8192 branch
set rtInfo["shell:lastcmd"] as already done and call
globalStore.set(termWrap.claudeCodeActiveAtom, false) (and any other related
atom/state if present) to explicitly reset the Claude-active flag; ensure this
mirrors the non-oversized path where isClaudeCodeCommand(decodedCmd) updates
claudeCodeActiveAtom and checkCommandForTelemetry is called.
---
Nitpick comments:
In `@frontend/app/view/term/osc-handlers.test.ts`:
- Around line 12-20: Tests currently miss two edge cases: commands prefixed with
"env KEY=... claude" and path-argument false positives like "echo
/usr/local/bin/claude". Update the test file (osc-handlers.test.ts) to add
assertions calling isClaudeCodeCommand("env ANTHROPIC_API_KEY=test claude")
expecting true and isClaudeCodeCommand("echo /usr/local/bin/claude") expecting
false; if these new tests fail, adjust the detection logic in
isClaudeCodeCommand to treat "env" style env assignments as valid invocations
and to ignore occurrences where "claude" appears as part of a path or as an
argument to echo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 93ef1ddc-6603-40ee-b32d-fa4c678d05c9
⛔ Files ignored due to path filters (2)
frontend/app/asset/claude-color.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/app/view/term/osc-handlers.test.tsfrontend/app/view/term/osc-handlers.tsfrontend/app/view/term/term-model.tsfrontend/app/view/term/term.tsxfrontend/app/view/term/termwrap.ts
| const ClaudeCodeRegex = /(?:^|\/)claude\b/; | ||
|
|
There was a problem hiding this comment.
Anchor Claude detection to the command token (argv[0]) only.
Line 28 currently matches /claude anywhere in the command string, so inputs like echo /usr/local/bin/claude are false positives.
🔧 Suggested fix
-const ClaudeCodeRegex = /(?:^|\/)claude\b/;
+const ClaudeCodeRegex = /^(?:\S*[\\/])?claude\b/;Also applies to: 91-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/view/term/osc-handlers.ts` around lines 28 - 29, The current
Claude detection uses ClaudeCodeRegex (/?:^|\/)claude\b/ ) against the entire
command string which produces false positives; update the code so detection runs
only against the command token (argv[0]) instead of the full command text —
i.e., extract the command token (e.g., const cmd = argv[0] or the variable
already holding the command name) and apply ClaudeCodeRegex to cmd, and make the
same change wherever ClaudeCodeRegex is used (including the other occurrence
around lines 91–96) so only the executable name/path is checked.
| function normalizeCmd(decodedCmd: string): string { | ||
| let normalizedCmd = decodedCmd.trim(); | ||
| normalizedCmd = normalizedCmd.replace(/^(?:\w+=(?:"[^"]*"|'[^']*'|\S+)\s+)*/, ""); | ||
| normalizedCmd = normalizedCmd.replace(/^env\s+/, ""); | ||
| return normalizedCmd; |
There was a problem hiding this comment.
normalizeCmd misses env KEY=... claude form.
Line 51 strips env , but assignment tokens after env are left in place, so Claude detection fails for common wrapper usage.
🔧 Suggested fix
function normalizeCmd(decodedCmd: string): string {
- let normalizedCmd = decodedCmd.trim();
- normalizedCmd = normalizedCmd.replace(/^(?:\w+=(?:"[^"]*"|'[^']*'|\S+)\s+)*/, "");
- normalizedCmd = normalizedCmd.replace(/^env\s+/, "");
- return normalizedCmd;
+ let normalizedCmd = decodedCmd.trim();
+ while (true) {
+ const prev = normalizedCmd;
+ normalizedCmd = normalizedCmd.replace(/^env\s+/, "");
+ normalizedCmd = normalizedCmd.replace(/^(?:\w+=(?:"[^"]*"|'[^']*'|\S+)\s+)*/, "");
+ normalizedCmd = normalizedCmd.trimStart();
+ if (normalizedCmd === prev) {
+ break;
+ }
+ }
+ return normalizedCmd;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function normalizeCmd(decodedCmd: string): string { | |
| let normalizedCmd = decodedCmd.trim(); | |
| normalizedCmd = normalizedCmd.replace(/^(?:\w+=(?:"[^"]*"|'[^']*'|\S+)\s+)*/, ""); | |
| normalizedCmd = normalizedCmd.replace(/^env\s+/, ""); | |
| return normalizedCmd; | |
| function normalizeCmd(decodedCmd: string): string { | |
| let normalizedCmd = decodedCmd.trim(); | |
| while (true) { | |
| const prev = normalizedCmd; | |
| normalizedCmd = normalizedCmd.replace(/^env\s+/, ""); | |
| normalizedCmd = normalizedCmd.replace(/^(?:\w+=(?:"[^"]*"|'[^']*'|\S+)\s+)*/, ""); | |
| normalizedCmd = normalizedCmd.trimStart(); | |
| if (normalizedCmd === prev) { | |
| break; | |
| } | |
| } | |
| return normalizedCmd; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/view/term/osc-handlers.ts` around lines 48 - 52, normalizeCmd
fails to strip forms like "env KEY=val claude" because it only removes a leading
"env " token; update normalizeCmd to remove an "env" wrapper plus any following
KEY=... assignments before the command. Specifically, modify the second
replacement (the one handling /^env\s+/ in normalizeCmd) to match
/^env(?:\s+\w+=(?:"[^"]*"|'[^']*'|\S+))*\s+/ so it strips "env" and zero-or-more
assignment tokens that follow, then returns the remaining command (so Claude
detection will see "claude" at the start).
| this.terminal.parser.registerOscHandler(7, (data: string) => { | ||
| return handleOsc7Command(data, this.blockId, this.loaded); | ||
| try { | ||
| return handleOsc7Command(data, this.blockId, this.loaded); | ||
| } catch (e) { | ||
| console.error("[termwrap] osc 7 handler error", this.blockId, e); | ||
| return false; | ||
| } | ||
| }); | ||
| this.terminal.parser.registerOscHandler(52, (data: string) => { | ||
| return handleOsc52Command(data, this.blockId, this.loaded, this); | ||
| try { | ||
| return handleOsc52Command(data, this.blockId, this.loaded, this); | ||
| } catch (e) { | ||
| console.error("[termwrap] osc 52 handler error", this.blockId, e); | ||
| return false; | ||
| } | ||
| }); | ||
| this.terminal.parser.registerOscHandler(16162, (data: string) => { | ||
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | ||
| try { | ||
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | ||
| } catch (e) { | ||
| console.error("[termwrap] osc 16162 handler error", this.blockId, e); | ||
| return false; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In xterm.js, for parser.registerOscHandler(code, handler), what is the exact behavior difference between returning trueandfalse from the handler callback?
💡 Result:
In xterm.js, for parser.registerOscHandler(code, handler), the handler is a callback that receives the OSC payload string and returns a boolean (or Promise). Returning true indicates the OSC sequence was successfully handled by this handler. The parser stops processing and does not invoke any further (earlier registered) handlers for this OSC code. Returning false (or any falsy value except undefined) indicates the sequence was not handled by this handler. The parser continues to the next handler in the list (handlers are tried from most recently added to earliest). Handlers are invoked in LIFO order (last in, first out). This allows later-registered handlers to intercept and handle the sequence exclusively by returning true, or pass it through by returning false to reach built-in or earlier handlers. This behavior is consistent across parser hooks (OSC, CSI, DCS, ESC).
Citations:
- 1: http://xtermjs.org/docs/guides/hooks/
- 2: http://xtermjs.org/docs/api/terminal/interfaces/iparser/
- 3: https://xtermjs.org/docs/guides/hooks/
- 4: https://cdn.jsdelivr.net/npm/xterm@4.6.0-beta.8/src/common/parser/OscParser.ts
🏁 Script executed:
# First, let's look at the file and surrounding context
head -220 frontend/app/view/term/termwrap.ts | tail -100Repository: wavetermdev/waveterm
Length of output: 4310
🏁 Script executed:
# Search for other OSC handler registrations for codes 7, 52, 16162
rg "registerOscHandler\((7|52|16162)" --context 3Repository: wavetermdev/waveterm
Length of output: 1640
🏁 Script executed:
# Look for the handler function definitions
fd "termwrap\|handleOsc" -e ts | head -20Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# Search for handleOsc7Command, handleOsc52Command, handleOsc16162Command
rg "handleOsc(7|52|16162)Command" -A 5Repository: wavetermdev/waveterm
Length of output: 3486
🏁 Script executed:
# Search for comments about OSC ownership or error handling
rg "OSC.*own|own.*OSC|handler.*own|own.*handler" -i -A 2 -B 2Repository: wavetermdev/waveterm
Length of output: 50377
Return true in catch blocks to maintain OSC handler ownership on errors.
Lines 181, 189, and 197 return false on exceptions, which contradicts the ownership behavior: the handlers themselves return true when not ready, signaling they own these OSC codes. Returning false on error allows other handlers to process sequences this handler already attempted, risking malformed data propagation.
Suggested fix
this.terminal.parser.registerOscHandler(7, (data: string) => {
try {
return handleOsc7Command(data, this.blockId, this.loaded);
} catch (e) {
console.error("[termwrap] osc 7 handler error", this.blockId, e);
- return false;
+ return true;
}
});
this.terminal.parser.registerOscHandler(52, (data: string) => {
try {
return handleOsc52Command(data, this.blockId, this.loaded, this);
} catch (e) {
console.error("[termwrap] osc 52 handler error", this.blockId, e);
- return false;
+ return true;
}
});
this.terminal.parser.registerOscHandler(16162, (data: string) => {
try {
return handleOsc16162Command(data, this.blockId, this.loaded, this);
} catch (e) {
console.error("[termwrap] osc 16162 handler error", this.blockId, e);
- return false;
+ return true;
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.terminal.parser.registerOscHandler(7, (data: string) => { | |
| return handleOsc7Command(data, this.blockId, this.loaded); | |
| try { | |
| return handleOsc7Command(data, this.blockId, this.loaded); | |
| } catch (e) { | |
| console.error("[termwrap] osc 7 handler error", this.blockId, e); | |
| return false; | |
| } | |
| }); | |
| this.terminal.parser.registerOscHandler(52, (data: string) => { | |
| return handleOsc52Command(data, this.blockId, this.loaded, this); | |
| try { | |
| return handleOsc52Command(data, this.blockId, this.loaded, this); | |
| } catch (e) { | |
| console.error("[termwrap] osc 52 handler error", this.blockId, e); | |
| return false; | |
| } | |
| }); | |
| this.terminal.parser.registerOscHandler(16162, (data: string) => { | |
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | |
| try { | |
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | |
| } catch (e) { | |
| console.error("[termwrap] osc 16162 handler error", this.blockId, e); | |
| return false; | |
| } | |
| }); | |
| this.terminal.parser.registerOscHandler(7, (data: string) => { | |
| try { | |
| return handleOsc7Command(data, this.blockId, this.loaded); | |
| } catch (e) { | |
| console.error("[termwrap] osc 7 handler error", this.blockId, e); | |
| return true; | |
| } | |
| }); | |
| this.terminal.parser.registerOscHandler(52, (data: string) => { | |
| try { | |
| return handleOsc52Command(data, this.blockId, this.loaded, this); | |
| } catch (e) { | |
| console.error("[termwrap] osc 52 handler error", this.blockId, e); | |
| return true; | |
| } | |
| }); | |
| this.terminal.parser.registerOscHandler(16162, (data: string) => { | |
| try { | |
| return handleOsc16162Command(data, this.blockId, this.loaded, this); | |
| } catch (e) { | |
| console.error("[termwrap] osc 16162 handler error", this.blockId, e); | |
| return true; | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/view/term/termwrap.ts` around lines 176 - 199, The OSC handler
catch blocks currently return false which relinquishes ownership on errors;
update the three handlers registered with registerOscHandler(7),
registerOscHandler(52), and registerOscHandler(16162) so their catch blocks
return true instead of false, keeping ownership when handleOsc7Command,
handleOsc52Command, or handleOsc16162Command throw; keep the existing logging
(console.error) and otherwise leave the try/catch behavior unchanged.
This updates the terminal shell-integration badge so it reflects Claude Code activity instead of always rendering the generic AI icon. When the active shell command is Claude Code, the header now shows the Claude logo.
Terminal shell-integration badge
getShellIntegrationIconButton()to render the Claude logo while Claude Code is the active running command.Claude Code detection
TermWrap, including initial runtime-info hydration and command lifecycle transitions.env.UI rendering
@lobehub/iconsand used itsClaudeicon in the terminal header path.Focused coverage
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.